• I think we should leave the splines function available for general use alongside your new spline_key_times function, rather than replacing it.

• Dunno about returning a Vec containing arbitrary iterators. Consider adding another type to the SplineGroup trait instead?

pub trait SplineGroup {
  type Sample;
  type KeyTimes;

  fn spline_key_times(&self) -> KeyTimes;

  // ...

}

impl SplineGroup for AnimationSplineOne {
  type Sample = Option<f32>;
  type KeyTimes = Box<dyn DoubleEndedIterator<Item = f32> + '_>;
  
  // ...

}

The thing is, there can no longer be an fn splines(&self) -> Vec<&Spline<f32, f32>> on SplineGroup, since not all splines have f32 values - one of them has Quats instead. So I looked around to see what the splines method was used for internally, replaced it with spline_key_times, and didn't really think too hard about how people might want to use it in general. I kind of assumed that it was just a side effect of needing to use it internally, and not having private trait methods. What's the use case?

I was absolutely not pleased with returning a Vec<Box<dyn Iterator>>, but I just wanted to get a proof of concept working. Now that you've brought it back to my attention, I would like to look into if there's a better way to implement start_time, end_time and is_empty on SplineGroup, which to my knowledge are the only things spline_key_times is used for. Maybe we could just leave them as required methods, instead of trying to create a general default implementation that will probably end up needing dynamic dispatch?

The thing is, there can no longer be an fn splines(&self) -> Vec<&Spline<f32, f32>> on SplineGroup, since not all splines have f32 values - one of them has Quats instead.

Ah, true that.

What's the use case?

main one I thought of is that if this is eventually integrated into a UI, you'd want direct access to the splines for drawing them in a curve editor.

Maybe we could just leave them as required methods, instead of trying to create a general default implementation that will probably end up needing dynamic dispatch?

Seems like a good option. Feel free to experiment.

It's worth mentioning that there's Another PR doing animation that uses property names and just seems overall more flexible than the hard-typed implementation we're doing here. It might be a better option entirely.

It's worth mentioning that there's Another PR doing animation that uses property names and just seems overall more flexible than the hard-typed implementation we're doing here. It might be a better option entirely.

Wow, I hadn't seen that. I'm looking through it now, trying to understand what they're doing.